Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Алешев Руслан #202

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

artBETEP
Copy link

using System.Text;

namespace ObjectPrinting
{
public class PrintingConfig<TOwner>
{
private readonly HashSet<PropertyInfo> excludedProperties = new HashSet<PropertyInfo>();
private readonly HashSet<Type> excludedTypes = new HashSet<Type>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю, можно воспользоваться короткой формой = new();. Если нет, то и нет, но можно поднять .net и версию C#

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="6.12.0" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай для тестов заводить отдельный проект, чтоб не мешать тесты и какую-либо логику. Плюсом в основном коде не будут доступны тестовые классы/методы (если правильно настроить область видимости), размер сборок станет меньше, проще будет запускать тесты отдельно и т.д.

Для Rider'а:
Добавляешь папку Tests: [имя твоего решения] -> клик ПКМ -> New Solution Project.
Делаешь вот так, указываешь нужное имя в Project name.
image

На выходе получается вот такое
image

{
excludedTypes.Add(typeof(TProp));
return this;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Перед сдачей решения запуская, пожалуйста, форматирование кода. Для Rider'a это сочетание ctrl + alt + L / shift + ctrl + alt + L. Либо можно настроить автоформатирование на сохранение кода ctrl s. Для Visual Studio не подскажу, но это гуглится. В коде оч много мест, где следовало бы применить форматирование.

public PrintingConfig<TOwner> Exclude<TProp>()
{
excludedTypes.Add(typeof(TProp));
return this;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Оставляй перед return пустую строку. И сразу проверь везде, пж)
Это делается для того, чтоб было сразу и четко ясно, что и где возвращается из метода.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

var values = dictionaryValue.Values.OfType<object>().Select(item => item.ToString()).ToArray();
var pair = keys.Zip(values).Select(pair => string.Format("({0}, {1})", pair.First, pair.Second));

return $"{propertyInfo.Name} = {{ " + string.Join(", ", pair) + " }";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут можно использовать интерполяцию целиком, без конкатенации.

var items = listValue.OfType<object>().Select(item => item.ToString()).ToArray();
return $"{propertyInfo.Name} = {{ " + string.Join(", ", items) + " }";
}

private string PrintToString(object obj, int nestingLevel)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется, что реализация сериализации должна лежать не в PrintingConfig

using FluentAssertions;
using System;
using System.Globalization;
using System.Collections.Generic;

namespace ObjectPrinting.Tests
{
[TestFixture]
public class ObjectPrinterAcceptanceTests
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нигде нет структуры ААА

[Test]
public void PrintToString_SkipExcludedPropertys()
{
var printer = ObjectPrinter.For<Person>().Exclude(x=>x.Age);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Часто тестируемый модуль называют sut - system under test. Так проще ориентироваться в тесте.

Вообще, ещё есть cut - class under system. Пишут так крайне редко (по крайней мере, исходя из моего опыта), но для полноты картинки пусть будет.

public void PrintToString_ParsinCyclingLinks()
{
var parent = new Person();
parent.Id = new Guid("00000000-0000-0000-0000-012147483648");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему бы не использовать инициализатор?
var parent = new Person {Id = new Guid("00000000-0000-0000-0000-012147483648")};?

{
var printer = ObjectPrinter.For<Person>().Serialize<double>().Using<Double>(CultureInfo.InvariantCulture);
var serialized = printer.PrintToString(person);
serialized.Should().Be("Person\r\n\tId = 00000000-0000-0000-0000-000000000000\r\n\tName = Alex\r\n\tHeight = 12.34\r\n\tAge = 19\r\n\tparent = null\r\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А можно ли применить культуру к всей сериализации? Т.е. сразу ко всем типам данных?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants